fix(cmd/gno): pass an ExecContext to MachineOptions in gno run
#2856
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses this issue: gnolang#2834 It tries to hew very closely to how the context is created under `gno test` so that there is consistency of results between code that is executed in the test and run contexts.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2856 +/- ##
==========================================
+ Coverage 60.92% 60.95% +0.02%
==========================================
Files 564 564
Lines 75267 75380 +113
==========================================
+ Hits 45854 45945 +91
- Misses 26041 26049 +8
- Partials 3372 3386 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
wyhaines
changed the title
gno run.gno run.
thehowl
reviewed
Sep 26, 2024
thehowl
reviewed
Sep 26, 2024
thehowl
changed the title
gno run.gno run
thehowl
approved these changes
Sep 27, 2024
moul
reviewed
Sep 27, 2024
| @@ -79,6 +79,10 @@ func TestRunApp(t *testing.T) { | |||
| args: []string{"run", "../../tests/integ/invalid_assign/main.gno"}, | |||
| recoverShouldContain: "cannot use bool as main.C without explicit conversion", | |||
| }, | |||
| { | |||
| args: []string{"run", "-expr", "Context()", "../../tests/integ/context/context.gno"}, | |||
| stdoutShouldContain: "Context worked", | |||
There was a problem hiding this comment.
would be nice to have the height so we can detect if it changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes: #2834
It tries to hew very closely to how the context is created under
gno testso that there is consistency of results between code that is executed in the test and run contexts. Please let me know if there are any conventions for the codebase that I should have followed that I did not.I hooked into the portion of the
ExecRun()that was callingNewMachineWithOptions(), and instead followed a similar pattern to what is defined intest.goto configure the machine with a context that has reasonable defaults. These, except for the Chain ID (which isdevwhen running undergno test), are identical to the test settings.This allows packages like
gno.land/p/demo/entropyto work when code is executed withgno run, as well as any others which might try to access information only available from the context.A simple piece of code to demonstrate the issue is below. This will fail without this change.
A test has been added to the tests for the run command, which tries to run code similar to the code above within the test.
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description